Conversation
273b407 to
587f7e5
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #713 +/- ##
==========================================
+ Coverage 84.92% 85.60% +0.67%
==========================================
Files 141 144 +3
Lines 2832 3007 +175
Branches 222 236 +14
==========================================
+ Hits 2405 2574 +169
- Misses 375 381 +6
Partials 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
docker/setup_configuration/data.yaml
Outdated
| '1': | ||
| - record__data__leeftijd | ||
| - record__data__kiemjaar | ||
| # TODO |
There was a problem hiding this comment.
removed the objecttype setup config thing but because of that you cannot really add permissions anymore, should it be added back and if yes only the required objecttype fields or everything?
There was a problem hiding this comment.
What's the reason for removing it? Is it because you can't create ObjectTypes now without specifying the JSON schema?
There was a problem hiding this comment.
Yes, the original use case was to only set the name and service so that all versions with their json schema's will be accesible, then in a tokenauth fields can be specified.
After the objecttype move, we could keep it as just the name, but then specifying the tokenauth permission fields becomes weird because that the schema cannot be defined in in the setup conf.
So i would propose to either:
- remove objecttypes from setup config.
- add the bare minimum (name, name_plural is also required but can be derived) but remove the token auth permission fields.
- add version and schema to objecttype config but i feel like objecttype versions should not be created like this.
There was a problem hiding this comment.
Let's go for the second option for now
| help_text=_("Incremental index number of the object record."), | ||
| ) | ||
| object = models.ForeignKey(Object, on_delete=models.CASCADE, related_name="records") | ||
| version = models.PositiveSmallIntegerField( |
There was a problem hiding this comment.
version could become an FK?
docker/setup_configuration/data.yaml
Outdated
| '1': | ||
| - record__data__leeftijd | ||
| - record__data__kiemjaar | ||
| # TODO |
There was a problem hiding this comment.
What's the reason for removing it? Is it because you can't create ObjectTypes now without specifying the JSON schema?
|
@Floris272 will you do the docs in a separate PR? |
b4b24a2 to
d81a31c
Compare
| "service", | ||
| ], # TODO remove service from unique_fields after objecttype migration since it will no longer be part of the ObjectType model. |
There was a problem hiding this comment.
Command could be kept, Objecttypes will just be unique on their uuid only
yes |
5a4d32e to
d673439
Compare
stevenbal
left a comment
There was a problem hiding this comment.
Some small things remaining still, I'd recommend to split off the duplicate UUID stuff into a separate PR, because this PR has been open for quite a while already
src/objects/core/management/commands/check_for_external_objecttypes.py
Outdated
Show resolved
Hide resolved
docker/setup_configuration/data.yaml
Outdated
| '1': | ||
| - record__data__leeftijd | ||
| - record__data__kiemjaar | ||
| # TODO |
There was a problem hiding this comment.
Let's go for the second option for now
5e51c56 to
d9f47d4
Compare
|
| Branch | feature/564-open-objecten |
| Testbed | ubuntu-24.04 |
🚨 1 Alert
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| performance_test/tests/test_objects_list.py::test_objects_api_list_large_page_size_page_1 | Latency milliseconds (ms) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 334.81 ms(+5.38%)Baseline: 317.71 ms | 333.60 ms (100.36%) |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| performance_test/tests/test_objects_list.py::test_objects_api_list_filter_by_object_type | 📈 view plot 🚷 view threshold | 124.34 ms(+1.18%)Baseline: 122.89 ms | 129.04 ms (96.36%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_filter_one_result | 📈 view plot 🚷 view threshold | 18.61 ms(-7.12%)Baseline: 20.03 ms | 21.04 ms (88.46%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_large_page_size_page_1 | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 334.81 ms(+5.38%)Baseline: 317.71 ms | 333.60 ms (100.36%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_large_page_size_page_5 | 📈 view plot 🚷 view threshold | 327.65 ms(+4.03%)Baseline: 314.95 ms | 330.69 ms (99.08%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_small_page_size_page_20 | 📈 view plot 🚷 view threshold | 125.48 ms(-2.47%)Baseline: 128.66 ms | 135.09 ms (92.88%) |
|
@Floris272 could you rebase on master to fix the merge conflicts? |
…VERSION_CACHE_TIMEOUT
d9f47d4 to
d4e6f76
Compare
Fixes #564
Changes
TODO